Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quick-verify make rule. #48842

Merged
merged 2 commits into from
Jul 14, 2017
Merged

Conversation

enisoc
Copy link
Member

@enisoc enisoc commented Jul 12, 2017

This is useful for humans to run to catch obvious problems before
pushing commits and waiting for CI to run verify checks.

Quick mode only runs a whitelist of verify scripts that are reasonably fast.
I set the initial bar arbitrarily at <10s each on my workstation.

The whole set runs in <30s for me, assuming I had already run make and
hack/godep-restore.sh. This is compared to the full make verify
which takes [I don't know how long because I gave up after 45min].

Because of nounset, it was impossible to run without -v.
This is useful for humans to run to catch obvious problems before
pushing commits and waiting for CI to run verify checks.

Quick mode only runs a whitelist of verify scripts that are reasonably fast.
I set the initial bar arbitrarily at <10s each on my workstation.

The whole set runs in <30s for me, assuming I had already run `make` and
`hack/godep-restore.sh`. This is compared to the full `make verify`
which takes [I don't know how long because I gave up after 45min].
@enisoc enisoc added the release-note-none Denotes a PR that doesn't merit a release note. label Jul 12, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 12, 2017
@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 12, 2017
@CaoShuFeng
Copy link
Contributor

Test OK in my environment.
Thanks.
LGTM

@cblecker
Copy link
Member

cross-build is currently broken: #48887

@cblecker
Copy link
Member

Looks great. Thanks for this @enisoc !
@kubernetes/sig-contributor-experience-misc: FYI

/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. labels Jul 13, 2017
@enisoc
Copy link
Member Author

enisoc commented Jul 13, 2017

/assign @ixdy

for OWNERS review.

@cblecker
Copy link
Member

/test pull-kubernetes-cross

@ixdy
Copy link
Member

ixdy commented Jul 14, 2017

/lgtm

nice!

@ixdy
Copy link
Member

ixdy commented Jul 14, 2017

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, enisoc, ixdy

Associated issue requirement bypassed by: ixdy

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 048b060 into kubernetes:master Jul 14, 2017
@enisoc enisoc deleted the quick-verify branch July 17, 2017 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants